London | ITP-May-26 | Vito Moratti | Sprint 3 | Project TV Show - #96
London | ITP-May-26 | Vito Moratti | Sprint 3 | Project TV Show#96vmoratti wants to merge 39 commits into
Conversation
London | ITP-May-26 | Vito Moratti | SPRINT 3 | Project TV Show
London | 26-ITP-May | Gideon Defar | Sprint 3 |Project-TV-Show Level-200
London | ITP-May-26 | Vito Moratti | Sprint 3 |Project TV Show-refactored
London | ITP-May-26 | Vito Moratti | Sprint 3 | Project TV Show/Feature/level 300
London | ITP-May-26 | Gideon Defar | Sprint 3 | Project TV Show -Level-400
London | ITP-May-26 | Vito Moratti | Sprint 3 | Project TV Show/level 500
vmoratti
added
the
Needs Review
London | ITP-May-26 | Vito Moratti | Project TV Show-Feature/level 500
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work on this. Your code is well organised. All the functionality is met, and good use of caching as needed.
I've spotted a few things, so if you want an extra challenge now ITP is over, see if you can address them.
There was a problem hiding this comment.
Good work trying to use semantic HTML,but there's a few issues in how you are using it. Have a look at https://validator.w3.org/nu/?doc=https%3A%2F%2Fvmoratti.github.io%2FProject-TV-Show%2F to get some pointers
| <!-- Level 400: choose a show from the full list --> | ||
| <section class="show-selector-control"> | ||
| <select id="show-select"> | ||
| <option value="">Select a show</option> |
There was a problem hiding this comment.
As well as the validator tool, have a look at the lighthouse tool, and see if it has any suggestions about accessibility and select elements.
| const footer = document.querySelector("footer"); | ||
|
|
||
| // Add the TVMaze link to the footer | ||
| footer.innerHTML = |
There was a problem hiding this comment.
Is there a reason you set the footer in javascript rather than the HTML, which has a different footer?
| Remove the tags so that only the text is displayed. | ||
| */ | ||
| function cleanSummary(summary) { | ||
| return (summary || "").replace(/<[^>]*>/g, ""); |
There was a problem hiding this comment.
Good idea to consider this!
| } | ||
|
|
||
| /* Styling for the link used to return to the shows page */ | ||
| nav a { |
There was a problem hiding this comment.
Does this make it obvious enough that this is a clickable control?
| // If there is no image, remove the empty image element | ||
| if (episode.image && episode.image.medium) { | ||
| image.src = episode.image.medium; | ||
| image.alt = `${episode.name} poster`; |
There was a problem hiding this comment.
Is this the best way to use this alt tag? See what https://axesslab.com/alt-texts/ has to say
LonMcGregor
added
Reviewed
3 participants
Self checklist
Completed Project TV Show
Link to live website:
https://vmoratti.github.io/Project-TV-Show/
Link to my partner's live wwebsite:
https://gideondefar.github.io/Project-TV-Show/